Fix: Reset block production executor state after chain reorg #857
Conversation
heemankv
changed the title
Mohiiit
left a comment
Mohiiit
left a comment
There was a problem hiding this comment.
what would happen to the l1->l2 deposits? will it get reverted in the bridge as well?
what about withdrawals? (Ik we will have a window of some time before that could happen)
is this for replay-service only?
I think revertTO in sequencer mode should specify the assumption it's making
| // Reset the executor state to NewBlock with the fresh state adapter | ||
| state = ExecutorThreadState::NewBlock(ExecutorStateNewBlock { | ||
| state_adaptor: new_state_adaptor, | ||
| consumed_l1_to_l2_nonces: HashSet::new(), |
There was a problem hiding this comment.
what would happen to the nonces that were in the hasmap earlier? will those nonces get saved in the db?
| let latest_block_n = new_state_adaptor.previous_block_n(); | ||
| tracing::info!("✅ State adapter reinitialized to block_n={}", new_state_adaptor.block_n()); |
There was a problem hiding this comment.
latest_block_n and block_n() are different blocks?
|
|
||
| *.rs | ||
| *.toml | ||
| Cargo.toml | ||
| Cargo.lock |
There was a problem hiding this comment.
so prettier is for .js, .md files only?
| let bp_status = self.ctx.service_status(MadaraServiceId::BlockProduction); | ||
| if matches!(bp_status, mp_utils::service::MadaraServiceStatus::On) { | ||
| if let Some(block_prod_handle) = &self.block_prod_handle { | ||
| tracing::debug!("revertTo: resetting block production state"); | ||
| block_prod_handle | ||
| .reset_state() | ||
| .await | ||
| .context("Resetting block production executor state after reorg") | ||
| .map_err(StarknetRpcApiError::from)?; | ||
| tracing::debug!("revertTo: block production state reset complete"); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
this is business logic, should not live in the rpc crate
|
|
||
| // Reset block production executor state if block production is enabled | ||
| let bp_status = self.ctx.service_status(MadaraServiceId::BlockProduction); | ||
| if matches!(bp_status, mp_utils::service::MadaraServiceStatus::On) { |
There was a problem hiding this comment.
why do we need to check if service is on?
| super::ExecutorCommand::ResetState(callback) => { | ||
| tracing::info!("🔄 Resetting executor state after reorg"); | ||
| // Reinitialize the state adapter from the current database state | ||
| let new_state_adaptor = match mc_exec::LayeredStateAdapter::new(self.backend.clone()) { |
There was a problem hiding this comment.
we can just do state = initial_state(), send the message StateReset and return from callback. all resetting of values isn't needed imo.
|
as discussed, let's consider switching BPS off and on inside revertTo itself |
Fix: Reset block production executor state after chain reorg
Pull Request Type
Problem Statement
When the
revertToRPC method was called while Madara was actively in block production state, the system encountered errors. The root cause was that the block production executor maintained stale state after the database was reverted:Solution
Implemented a state reset mechanism for the block production executor that triggers after chain reorganizations:
Core Changes
Added Reset Command: Introduced a new
ResetStateexecutor command that:NewBlockstatecontinueto naturally re-enter the wait loop for new transactionsRPC Integration: The
revertToRPC method now callsreset_state()on the block production handle after reverting the chain, ensuring the executor state stays synchronized with the database state.State Synchronization: Added
StateResetmessage to notify the block production task when the executor has been reset, allowing proper state machine transitions.Additional Improvements
get_custom_header()methods toimpl<D: MadaraStorageRead>block to make them available for all storage trait bounds, eliminating manual mutex lock patterns throughout the codebaseDoes this introduce a breaking change?
No
Other Information
The fix ensures that block production can safely resume after any chain reorganization triggered by the
revertToRPC, with all executor state properly synchronized to the new chain tip.